Prevent script injection in GitHub Actions workflows - #2355
Conversation
Untrusted GitHub context (workflow_dispatch inputs, PR author login, github.actor) was interpolated directly into inline run: scripts, allowing shell script injection into the runner. Bind these values to env: variables and reference them as quoted shell variables so they are never evaluated as shell code. Fixes ACAT finding for go-release.yml and smithy-diff.yml. sim: https://t.corp.amazon.com/V2263112241
|
Detected changes to the release files or to the check-files action |
|
Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS |
lucasmcdonald3
left a comment
lucasmcdonald3
left a comment
There was a problem hiding this comment.
LGTM
Style note, I'd appreciate it if you could add a prompt to your agents to follow our PR templates and ideally limit the amount of content they write there, I'm not reading all of that (https://noslopgrenade.com/)
There was a problem hiding this comment.
Pull request overview
This PR mitigates script-injection risks in GitHub Actions workflows by preventing untrusted GitHub context values from being interpolated directly into shell run: blocks, instead routing them through env: and quoted shell variable references.
Changes:
- Refactors
smithy-diff.ymlto bind PR/user/repo identifiers viaenv:and use them in therun:script via shell variables. - Refactors
go-release.ymlto bindproject-name/versioninputs viaenv:and quote their usage in shell commands. - Quotes certain shell arguments/paths to reduce unintended shell parsing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/smithy-diff.yml | Moves GitHub context fields into env: and references them as shell variables in the comment/curl step. |
| .github/workflows/go-release.yml | Moves workflow_dispatch inputs into env: and quotes their usage in release automation steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| chmod +x ./submodules/MaterialProviders/scripts/go-release-automation.sh | ||
| RELEASE_DIR_NAME=$(./submodules/MaterialProviders/scripts/go-release-automation.sh get_release_dir_name "${{ github.event.inputs.project-name }}" "${{ github.event.inputs.version }}") | ||
| RELEASE_DIR_NAME=$(./submodules/MaterialProviders/scripts/go-release-automation.sh get_release_dir_name "$PROJECT_NAME" "$VERSION") | ||
| echo "releaseDirName=$RELEASE_DIR_NAME" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Fixed in 804dd51: releaseDirName is now written using the documented heredoc-delimiter form and $GITHUB_OUTPUT is quoted.
| COMMENT="@${PR_USER} and @${ACTOR}, I noticed you are updating the smithy model files.\nDoes this update need new or updated javadoc trait documentation?\n Are you adding constraints inside list, map or union? Do you know about this issue: https://github.com/smithy-lang/smithy-dafny/issues/491?" | ||
| COMMENT_URL="https://api.github.com/repos/${REPO}/issues/${PR_NUMBER}/comments" | ||
| curl -s -H "Authorization: token ${GITHUB_TOKEN}" -X POST "$COMMENT_URL" -d "{\"body\":\"$COMMENT\"}" |
There was a problem hiding this comment.
Fixed in 804dd51: the JSON body is now built with jq -nc --arg body "$COMMENT" '{body: $body}', which safely encodes quotes, backslashes, and newlines.
Address PR review feedback: - Write releaseDirName via heredoc delimiter and quote $GITHUB_OUTPUT to avoid step-output injection. - Build the PR comment JSON body with jq so special characters are safely encoded. sim: https://t.corp.amazon.com/V2263112241
|
Detected changes to the release files or to the check-files action |
|
Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS |
aws-lc-rs 1.17.3 now resolves aws-lc-sys to 0.43.0, while the direct dependency was pinned to 0.42, causing two copies of AWS-LC to resolve and failing the duplicate-aws-lc CI check. Align the direct pin to 0.43. The fips profile (aws-lc-fips-sys 0.13.1) is unaffected. sim: https://t.corp.amazon.com/V2263112241
|
Added commit eb6ffdd: bumped direct |
|
Detected changes to the release files or to the check-files action |
|
Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS |
|
Detected changes to the release files or to the check-files action |
Summary
Fixes the Script Injection in GitHub Actions workflows (V2263112241).
Untrusted GitHub context values were interpolated directly into inline
run:shell scripts, which allows an attacker-controlled value to be evaluated as shell code on the runner. This change binds those values toenv:variables and references them as quoted shell variables, so they are treated as data rather than executable script.Changes
.github/workflows/go-release.ymlGet release directory name—project-name/versioninputs moved toenv($PROJECT_NAME,$VERSION).Run Go release automation script— same, and arguments quoted.print diff...—project-nameandreleaseDirNamemoved toenv; paths quoted..github/workflows/smithy-diff.ymlCheck if FILES is not empty—pull_request.user.login,github.actor, andgithub.repositorymoved toenv($PR_USER,$ACTOR,$REPO) and referenced as shell variables in the comment/curlbody.Testing
${{ github.event* }},${{ github.actor }}, or${{ steps.* }}interpolations remain inside anyrun:block — all such values are now inenv:blocks (the recommended safe pattern).Notes
Generate a changelogstep usesreleaseDirNamein awith: args:(an action input, not arun:shell body) and is workflow-generated, so it was not part of the finding and is left unchanged.sim: https://t.corp.amazon.com/V2263112241